Client refactor #88

0bsearch · 2018-06-04T17:27:15Z

Fixes

Checklist

I have made a material change to the repo (functionality, testing, spelling, grammar)
I have read the [Contribution Guide] and my PR follows them.
I updated my branch with the master branch.
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation about the functionality in the appropriate .md file
I have added in line documentation to the code I modified

Short description of what this PR does:

Better signatures for client; minor refactoring

SendGridDX · 2018-06-04T17:27:33Z

All committers have signed the CLA.

codecov · 2018-06-04T17:37:48Z

Codecov Report

Merging #88 into master will increase coverage by 1.72%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   91.46%   93.19%   +1.72%     
==========================================
  Files           6        6              
  Lines         293      294       +1     
==========================================
+ Hits          268      274       +6     
+ Misses         25       20       -5

Impacted Files	Coverage Δ
tests/test_unit.py	`99.26% <100%> (+0.06%)`	⬆️
python_http_client/client.py	`80.64% <50%> (+2.97%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa72b74...8812835. Read the comment docs.

thinkingserious · 2018-06-05T02:10:15Z

Thank you @3lnc! I've added this to our backlog for a code review.

42B · 2018-10-28T00:50:34Z

python_http_client/client.py

@@ -62,6 +63,9 @@ def to_dict(self):
 class Client(object):
    """Quickly and easily access any REST or REST-like API."""

+    # These are the supported HTTP verbs
+    methods = set(('delete', 'get', 'patch', 'post', 'put'))


set literal instead?

methods = {'delete', 'get', 'patch', 'post', 'put'}

At time this PR was created, http client claimed support for 2.6

@3lnc

Since that doesn't seem to be the case now... set literal?

These are the supported versions going forward: https://github.com/sendgrid/sendgrid-python/blob/v4/.travis.yml#L4

42B · 2018-10-28T00:52:40Z

tests/test_unit.py

@@ -84,7 +90,7 @@ def test__init__(self):
        self.assertEqual(default_client.host, self.host)
        self.assertEqual(default_client.request_headers, {})
        self.assertIs(default_client.timeout, None)
-        methods = ['delete', 'get', 'patch', 'post', 'put']
+        methods = set(('delete', 'get', 'patch', 'post', 'put'))


set literal?

methods = {'delete', 'get', 'patch', 'post', 'put'}

42B · 2018-10-28T00:52:57Z

tests/test_unit.py

@@ -97,7 +103,7 @@ def test__init__(self):
                        timeout=10)
        self.assertEqual(client.host, self.host)
        self.assertEqual(client.request_headers, request_headers)
-        methods = ['delete', 'get', 'patch', 'post', 'put']
+        methods = set(('delete', 'get', 'patch', 'post', 'put'))


set literal?

methods = {'delete', 'get', 'patch', 'post', 'put'}

42B · 2018-10-28T00:54:53Z

tests/test_unit.py

+        self.client._update_headers({'X-test': 'Test'})
+        self.client.get()
+        request = maker.call_args[0][1]
+        self.assertTrue('X-test' in request.headers)


self.assertIn('X-test', request.headers) ?

https://docs.python.org/3/library/unittest.html?highlight=assertin#unittest.TestCase.assertIn

Available since 3.1, client provides support for 2.7 (2.6 previously)

@3lnc

New in version 2.7.

https://docs.python.org/2/library/unittest.html?highlight=assertin#unittest.TestCase.assertIn

misterdorm · 2018-10-31T15:57:03Z

@3lnc Please address conflicts and code coverage failures. Thanks!

thinkingserious · 2018-11-14T20:39:27Z

Hi @3lnc,

When you get a moment to make the updates I'd love to merge this before we release v4 of the sendgrid-python library which will depend on this library.

Thanks!

With Best Regards,

Elmer

stewartjarod · 2019-06-13T03:09:02Z

python_http_client/client.py

-                    request.add_header('Content-Type', 'application/json')
+                request = urllib.Request(
+                    self._build_url(query_params),
+                    headers=self.request_headers,


This fixes an issue I am having when adding headers... request.add_header(key, value) capitalizes the header which some APIs don't seem to want. :-\

thinkingserious · 2019-06-14T04:03:37Z

Hello @3lnc,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

thinkingserious added the status: code review request requesting a community code review or review from Twilio label Jun 4, 2018

0bsearch force-pushed the client_refactor branch 2 times, most recently from 4c28722 to f656758 Compare June 4, 2018 18:05

thinkingserious added the difficulty: medium fix is medium in difficulty label Sep 28, 2018

42B reviewed Oct 28, 2018

View reviewed changes

misterdorm added status: waiting for feedback waiting for feedback from the submitter and removed status: code review request requesting a community code review or review from Twilio labels Oct 31, 2018

thinkingserious added the type: community enhancement feature request not on Twilio's roadmap label Nov 14, 2018

Adds explicit http_request args

749f22c

0bsearch force-pushed the client_refactor branch from 4313ca7 to bdb061c Compare November 15, 2018 13:31

Pulls HTTP methods to class attr

22e6d1e

0bsearch force-pushed the client_refactor branch from bdb061c to 22e6d1e Compare November 15, 2018 13:34

Adds mock to travis deps

8812835

stewartjarod reviewed Jun 13, 2019

View reviewed changes

stewartjarod approved these changes Jun 13, 2019

View reviewed changes

thinkingserious merged commit 36b241f into sendgrid:master Jun 14, 2019

Client refactor #88

Client refactor #88

Uh oh!

Conversation

0bsearch commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes

Checklist

Short description of what this PR does:

Uh oh!

SendGridDX commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thinkingserious commented Jun 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

misterdorm commented Oct 31, 2018

Uh oh!

thinkingserious commented Nov 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thinkingserious commented Jun 14, 2019

Uh oh!

Uh oh!

0bsearch commented Jun 4, 2018 •

edited

Loading

SendGridDX commented Jun 4, 2018 •

edited

Loading

codecov bot commented Jun 4, 2018 •

edited

Loading